Skip to content

Fix search failure on user index due to nested owners query#26794

Open
sonika-shah wants to merge 4 commits intomainfrom
nested-owners-query-ignore-unmapped
Open

Fix search failure on user index due to nested owners query#26794
sonika-shah wants to merge 4 commits intomainfrom
nested-owners-query-ignore-unmapped

Conversation

@sonika-shah
Copy link
Collaborator

@sonika-shah sonika-shah commented Mar 26, 2026

Describe your changes:

Fixes #3280

The RBAC layer unconditionally adds nested owners queries to all search
requests. Indexes like user that don't have owners as a nested field
fail with "failed to find nested object under path [owners]".

Add ignore_unmapped=true to all nested queries so they gracefully return
no matches on indexes without the nested field instead of throwing 500.

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

@sonika-shah sonika-shah requested a review from a team as a code owner March 26, 2026 10:24
@sonika-shah sonika-shah added safe to test Add this label to run secure Github workflows on PRs To release Will cherry-pick this PR into the release branch labels Mar 26, 2026
Copilot AI review requested due to automatic review settings March 26, 2026 10:24
@sonika-shah sonika-shah removed the To release Will cherry-pick this PR into the release branch label Mar 26, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes search failures when RBAC injects owners nested queries against indexes where owners is not mapped as a nested field by adding ignore_unmapped=true to nested queries (Elasticsearch + OpenSearch), with accompanying test coverage.

Changes:

  • Add ignore_unmapped to nested query builders for Elasticsearch and OpenSearch.
  • Add ignore_unmapped to JSON-based nested query filters (e.g., owners filters) and assert it in unit tests.
  • Re-enable a previously skipped Playwright E2E test related to notification alerts.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/NotificationAlerts.spec.ts Unskips an alerts permissions E2E flow.
openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/queries/OpenSearchQueryBuilder.java Sets ignoreUnmapped(true) on nested queries built via the OpenSearch query builder.
openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchQueryBuilder.java Sets ignoreUnmapped(true) on the static OpenSearch nested query helper.
openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/queries/ElasticQueryBuilder.java Sets ignoreUnmapped(true) on nested queries built via the Elasticsearch query builder.
openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticQueryBuilder.java Sets ignoreUnmapped(true) on the static Elasticsearch nested query helper.
openmetadata-service/src/main/java/org/openmetadata/service/search/SearchListFilter.java Adds ignore_unmapped:true to the owners nested filter JSON.
openmetadata-service/src/main/java/org/openmetadata/service/search/QueryFilterBuilder.java Adds ignore_unmapped:true to nested query JSON generated by filter builder helpers.
openmetadata-service/src/test/java/org/openmetadata/service/search/security/OpenSearchRBACConditionEvaluatorTest.java Adds tests asserting nested query JSON includes ignore_unmapped:true (OpenSearch).
openmetadata-service/src/test/java/org/openmetadata/service/search/security/ElasticSearchRBACConditionEvaluatorTest.java Adds tests asserting nested query JSON includes ignore_unmapped:true (Elasticsearch).
openmetadata-service/src/test/java/org/openmetadata/service/search/QueryFilterBuilderTest.java Asserts ignore_unmapped is present in nested filters and adds additional nested-query structure checks.
openmetadata-service/src/test/java/org/openmetadata/service/jdbi3/SearchListFilterTest.java Asserts ignore_unmapped is present in owners nested filters and updates expected serialized content.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 26, 2026

OpenMetadata Service New-Code Coverage

PASS. Required changed-line coverage: 90.00% overall and per touched production file.

  • Overall executable changed lines: 7/7 covered (100.00%)
  • Missed executable changed lines: 0
  • Non-executable changed lines ignored by JaCoCo: 1
  • Changed production files: 6
File Covered Missed Executable Non-exec Coverage Uncovered lines
openmetadata-service/src/main/java/org/openmetadata/service/search/QueryFilterBuilder.java 3 0 3 0 100.00% -
openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticQueryBuilder.java 1 0 1 0 100.00% -
openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/queries/ElasticQueryBuilder.java 1 0 1 0 100.00% -
openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchQueryBuilder.java 1 0 1 0 100.00% -
openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/queries/OpenSearchQueryBuilder.java 1 0 1 0 100.00% -
openmetadata-service/src/main/java/org/openmetadata/service/search/SearchListFilter.java 0 0 0 1 N/A -

Only changed executable lines under openmetadata-service/src/main/java are counted. Test files, comments, imports, and non-executable lines are excluded.

@github-actions
Copy link
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 64%
64.85% (58126/89627) 44.67% (30722/68764) 47.69% (9199/19289)

@sonarqubecloud
Copy link

@github-actions
Copy link
Contributor

github-actions bot commented Mar 26, 2026

🟡 Playwright Results — all passed (24 flaky)

✅ 3392 passed · ❌ 0 failed · 🟡 24 flaky · ⏭️ 216 skipped

Shard Passed Failed Flaky Skipped
🟡 Shard 1 450 0 5 2
🟡 Shard 2 598 0 6 32
🟡 Shard 3 604 0 5 27
🟡 Shard 4 598 0 5 47
✅ Shard 5 587 0 0 67
🟡 Shard 6 555 0 3 41
🟡 24 flaky test(s) (passed on retry)
  • Features/DataAssetRulesDisabled.spec.ts › Verify the Pipeline entity item action after rules disabled (shard 1, 1 retry)
  • Flow/Tour.spec.ts › Tour should work from help section (shard 1, 1 retry)
  • Flow/Tour.spec.ts › Tour should work from welcome screen (shard 1, 1 retry)
  • Flow/Tour.spec.ts › Tour should work from URL directly (shard 1, 1 retry)
  • Pages/UserCreationWithPersona.spec.ts › Create user with persona and verify on profile (shard 1, 1 retry)
  • Features/BulkEditEntity.spec.ts › Glossary (shard 2, 1 retry)
  • Features/DataQuality/TestCaseImportExportE2eFlow.spec.ts › Admin: Complete export-import-validate flow (shard 2, 1 retry)
  • Features/DataQuality/TestCaseImportExportE2eFlow.spec.ts › EditAll User: Complete export-import-validate flow (shard 2, 1 retry)
  • Features/DataQuality/TestCaseIncidentPermissions.spec.ts › User with TEST_CASE.EDIT_ALL can see edit icon on incidents (shard 2, 1 retry)
  • Features/DataQuality/TestCaseIncidentPermissions.spec.ts › User with only VIEW cannot PATCH incidents (shard 2, 1 retry)
  • Features/DataQuality/TestCaseResultPermissions.spec.ts › User with TEST_CASE.EDIT_ALL can see edit action on test case (shard 2, 1 retry)
  • Features/Permissions/GlossaryPermissions.spec.ts › Team-based permissions work correctly (shard 3, 1 retry)
  • Flow/AddRoleAndAssignToUser.spec.ts › Verify assigned role to new user (shard 3, 1 retry)
  • Flow/ExploreDiscovery.spec.ts › Should display deleted assets when showDeleted is checked and deleted is not present in queryFilter (shard 3, 1 retry)
  • Flow/PersonaFlow.spec.ts › Set default persona for team should work properly (shard 3, 1 retry)
  • Flow/SchemaTable.spec.ts › schema table test (shard 3, 1 retry)
  • Pages/Customproperties-part2.spec.ts › entityReferenceList shows item count, scrollable list, no expand toggle (shard 4, 1 retry)
  • Pages/DataContracts.spec.ts › Create Data Contract and validate for Table (shard 4, 1 retry)
  • Pages/DataContracts.spec.ts › Add and update Security and SLA tabs (shard 4, 1 retry)
  • Pages/DataContractsSemanticRules.spec.ts › Validate Description Rule Is_Set (shard 4, 1 retry)
  • Pages/Domains.spec.ts › Rename domain with data products attached at domain and subdomain levels (shard 4, 1 retry)
  • Pages/Glossary.spec.ts › Column dropdown drag-and-drop functionality for Glossary Terms table (shard 6, 1 retry)
  • Pages/ServiceEntity.spec.ts › Tier Add, Update and Remove (shard 6, 1 retry)
  • Pages/Users.spec.ts › Permissions for table details page for Data Consumer (shard 6, 1 retry)

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

Copilot AI review requested due to automatic review settings March 27, 2026 14:46
@gitar-bot
Copy link

gitar-bot bot commented Mar 27, 2026

Code Review ✅ Approved

Fixes search failure on the user index caused by nested owners query by ignoring unmapped fields. No issues found.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Comment on lines 151 to 159
private String getOwnerCondition() {
String owners = getQueryParam("owners");
if (!nullOrEmpty(owners)) {
String ownersList =
Arrays.stream(owners.split(",")).collect(Collectors.joining("\", \"", "\"", "\""));
return String.format(
"{\"nested\":{\"path\":\"owners\",\"query\":{\"terms\":{\"owners.id\":[%s]}}}}",
"{\"nested\":{\"path\":\"owners\",\"query\":{\"terms\":{\"owners.id\":[%s]}},\"ignore_unmapped\":true}}",
ownersList);
}
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description says to add ignore_unmapped=true to all nested queries, but in this class only the owners nested query was updated. There are other nested query JSON snippets built here (e.g., tags and testSuites) that still omit ignore_unmapped, so similar unmapped-field failures could still occur and the change is inconsistent with the stated intent. Consider adding ignore_unmapped to those nested clauses as well (and updating tests accordingly).

Copilot uses AI. Check for mistakes.
@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants